• packages/schema/src/cli/cli-util.ts (2 hunks)
• packages/schema/src/plugins/prisma/schema-generator.ts (2 hunks)
• packages/schema/src/utils/ast-utils.ts (2 hunks)
• packages/schema/src/utils/pkg-utils.ts (3 hunks)

packages/schema/src/utils/ast-utils.ts (2)

• 21-21: The addition of the findNodeModulesFile import from pkg-utils is necessary for the new functionality introduced in the resolveImportUri function. This change aligns with the PR's objective to enhance import capabilities, specifically for handling imports from node_modules.
• 98-114: The modifications to the resolveImportUri function introduce several key enhancements:

• Adding a .zmodel extension to paths that don't already end with it.
• Handling different path formats (relative, absolute Unix, absolute Windows).
• Utilizing findNodeModulesFile for resolving paths within node_modules.

These changes are crucial for supporting the feature of importing models from node_modules. However, there are a few considerations:

• The check for absolute Windows paths uses a regex that specifically looks for paths starting with a drive letter followed by :\\. Ensure this regex accurately captures all valid Windows absolute paths.
• The use of findNodeModulesFile assumes that if a path does not start with ., /, or match the Windows absolute path regex, it must be a module within node_modules. This logic seems sound, but it's important to verify that this assumption holds in all intended use cases.
• The function now mutates imp.path directly by appending .zmodel if necessary. While this should work as intended, consider the implications of mutating function arguments and whether this could have unintended side effects elsewhere.

Overall, these changes significantly improve the flexibility and capability of the resolveImportUri function, aligning with the PR's goals.

packages/schema/src/utils/pkg-utils.ts (3)

• 32-40: The refactoring of the findUp function introduces support for searching multiple paths and improves clarity and extensibility. This change is significant for the PR's objective to enhance path resolution capabilities. The function now supports returning either a single path or an array of paths, depending on the multiple flag. This flexibility is crucial for locating files within nested directories, such as node_modules. The implementation appears to be correct and efficiently uses recursion to traverse up the directory tree. However, ensure thorough testing, especially for edge cases like symbolic links or permission issues in certain directories.
• 56-65: The addition of the findNodeModulesFile function is a key part of the PR's objective to enable importing from node_modules. This function uses the enhanced findUp to locate the node_modules directory and then attempts to find the specified file within it. The logic for splitting the provided name into folder and file parts and constructing the file location is sound. However, consider adding more detailed error handling or logging to help diagnose issues when files cannot be found. Additionally, ensure that this function is thoroughly tested with various directory structures and naming conventions within node_modules.
• 136-140: The deprecation notice for findPackageJson in favor of using findUp is a good practice, as it encourages the use of the more flexible and generalized findUp function. This change aligns with the PR's goal of improving path resolution logic. Ensure that all references to findPackageJson within the project have been updated to use findUp, and consider adding a warning log or console output when findPackageJson is used, to alert developers of the deprecation.

packages/schema/src/cli/cli-util.ts (1)

• 283-283: The replacement of findPackageJson with findUp in the getDefaultSchemaLocation function is a direct application of the changes made in pkg-utils.ts. This update leverages the enhanced capabilities of findUp to locate the package.json file, which is a more flexible and robust approach than the previous findPackageJson function. This change is consistent with the PR's objectives and improves the logic for schema location retrieval. Ensure that the rest of the codebase has been updated accordingly to use findUp where findPackageJson was previously used.

packages/schema/src/plugins/prisma/schema-generator.ts (2)

• 51-51: The import statement for findUp from ../../utils/pkg-utils replaces the previous findPackageJson. This change aligns with the PR's objective to enhance path resolution logic, particularly for locating package.json files. Ensure that all instances where findPackageJson was used are now correctly utilizing findUp with the appropriate arguments and handling its return values correctly.
• 453-453: The usage of findUp to locate package.json in the function getDefaultPrismaOutputFile is a direct application of the newly imported utility. This change is crucial for the feature allowing users to import models from node_modules by improving the path resolution logic. It's important to verify that the findUp function is correctly implemented to handle both the search for package.json and the resolution of the Prisma schema output file path based on the zenstack.prisma configuration within package.json. Additionally, ensure that error handling and edge cases, such as the absence of package.json or invalid configurations, are adequately addressed.

• packages/schema/src/utils/ast-utils.ts (2 hunks)
• packages/schema/src/utils/pkg-utils.ts (3 hunks)

packages/schema/src/utils/ast-utils.ts (1)

• 21-21: The addition of findNodeModulesFile from pkg-utils is appropriate for the new functionality introduced in resolveImportUri. This change aligns with the PR's objective to enhance import capabilities from node_modules.

packages/schema/src/utils/pkg-utils.ts (1)

• 135-135: The deprecation of findPackageJson in favor of findUp is noted. This change aligns with the PR's objective to refine type definitions and improve functionality. Ensure that all references to findPackageJson are updated to use findUp where applicable.

Verification successful

Given the findings, it's necessary to further investigate whether the remaining reference to findPackageJson is actively used in the codebase or if it's simply its definition. To clarify this, I will search for calls to findPackageJson excluding its definition.

---

The verification process reveals that while findPackageJson is still defined in the codebase, there are no active calls to this function outside of its definition. This supports the review comment's implication that findPackageJson has been deprecated in favor of findUp, aligning with the PR's objective to refine type definitions and improve functionality. Therefore, the review comment is consistent with the findings in the codebase.

Scripts Executed

The following scripts were executed for the analysis:

---

Script:

#!/bin/bash
# Search for any remaining references to findPackageJson in the codebase.
rg --type ts 'findPackageJson'

Length of output: 124

---

---

Script:

#!/bin/bash
# Search for calls to findPackageJson excluding its definition
rg --type ts 'findPackageJson' | grep -v 'export function findPackageJson'

Length of output: 74

• packages/schema/src/utils/ast-utils.ts (2 hunks)

packages/schema/src/utils/ast-utils.ts (2)

• 21-21: The addition of findNodeModulesFile from pkg-utils is a crucial part of enabling imports from node_modules. Ensure that pkg-utils is properly versioned and maintained to avoid potential future compatibility issues.
• 98-113: The modifications to resolveImportUri introduce more robust handling of import paths, including support for node_modules paths. However, there are several points to consider:

• The addition of the .zmodel extension (lines 100-101) assumes all imports should end with this extension. Confirm this assumption holds for all use cases.
• The logic to determine if a path points to node_modules (lines 104-110) is clear. Ensure findNodeModulesFile gracefully handles cases where the node_modules directory or the specified module does not exist.
• The final resolution of the path (line 113) is straightforward. Ensure that error handling is in place for cases where the resolved path does not exist or is inaccessible.

+ try {
+     const resolvedPath = Utils.resolvePath(dirUri, imp.path);
+     if (fs.existsSync(resolvedPath)) {
+         return resolvedPath;
+     } else {
+         throw new Error(`Resolved path does not exist: ${resolvedPath}`);
+     }
+ } catch (error) {
+     console.error(`Error resolving import path: ${error}`);
+     return undefined;
+ }

• Consider using path.isAbsolute for checking absolute paths, as suggested by ymc9 in previous comments. This could simplify the checks for absolute paths on different platforms.

- && !imp.path.startsWith('/') // Respect absolute paths (Unix)
- && !/^[a-zA-Z]:\\/.test(imp.path) // Respect absolute paths (Windows)
+ && !path.isAbsolute(imp.path) // Respect absolute paths

• packages/schema/src/utils/ast-utils.ts (2 hunks)
• packages/schema/src/utils/pkg-utils.ts (3 hunks)

• packages/schema/src/utils/ast-utils.ts

packages/schema/src/utils/pkg-utils.ts (4)

• 8-9: The FindUp type definition is clear and well-documented. It effectively uses TypeScript's conditional types to return either a single string or an array of strings based on the generic parameter. This is a good use of TypeScript's advanced type features to create flexible and reusable utility types.
• 17-19: The findUp function is well-documented and appears to be a versatile utility for finding files or directories by traversing up the directory tree. The use of generics to allow for returning either a single path or an array of paths is a smart design choice. However, it's important to ensure that this function is thoroughly tested, especially in edge cases such as symbolic links or filesystem permissions issues.
• 40-52: The findNodeModulesFile function has been updated to use require.resolve for resolving module paths, which is a significant improvement over manual path construction. This leverages Node.js's own resolution logic, which is more reliable and handles edge cases better. The change addresses the comment from ymc9 regarding the use of require.resolve. This is a good example of incorporating feedback into the codebase.
• 134-134: The findPackageJson function is marked as deprecated in favor of the findUp function. This is a good move towards simplifying the codebase and reducing redundancy. However, it's important to ensure that all references to findPackageJson throughout the project have been updated to use findUp instead. Additionally, consider adding a deprecation warning in the function body to alert developers who might still be using it.

• packages/schema/src/utils/pkg-utils.ts (3 hunks)

• packages/schema/src/utils/pkg-utils.ts